Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDC multi-chunk contract loader #6237

Closed
wants to merge 12 commits into from
Closed

Conversation

vaivaswatha
Copy link
Contributor

With the target contract broken down into chunks (with sizes that are word aligned), we can load it in pieces and then execute.

On my local test, I split run_external_target contract binary into part-1.bin with size 2080 bytes and part-2.bin with size 2072 bytes. (Use command split -b 2080 ... on Linux for convenience).

With the below commands, the test run_external_can_proxy_call passed.

$cd test/src/sdk-harness
$forc build --release
$cargo test -- run_external_can_proxy_call --ignored --show-output

With the target contract broken down into chunks (with
sizes that are word aligned), we can load it in pieces
and then execute.
@vaivaswatha vaivaswatha self-assigned this Jul 6, 2024
@vaivaswatha vaivaswatha changed the title LDC multi-chunk contract LDC multi-chunk contract loader Jul 6, 2024
ironcev and others added 5 commits July 8, 2024 23:04
)

## Description

This PR adds `string slices` to `CURRENTLY_SUPPORTED_TYPES_MESSAGE` in
`match` expressions. The support for string slices is added in #6202.

## Checklist

- [x] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
## Description

This PR recover configurable tests that were deleted by mistake.

## Checklist

- [ ] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [ ] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Joshua Batty <joshpbatty@gmail.com>
## Description

This test was disabled when we introduced encoding v1. But it has been
fixed since and we can enable it again.


## Checklist

- [ ] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [ ] I have requested a review from the relevant team or maintainers.
## Description

Removes local snapshot we were using to fund our accounts as it is now
done by default as of FuelLabs/fuel-core#1894.
By default `--default-signer` is signing it with the account funded by
fuel-core out of the box.
## Description

This PR fixes a problem with `std::Bytes`, `std::Vec` and `std::String`
buffer ownership. It also fixes a problem with buffer overflow when
encoding huge types.

## Buffer Ownership

Currently, types like `Bytes`, `Vec` and `String` do not guarantee
ownership of their buffer. That means that we can very easily alias the
underlying buffer and change its mutability by simply creating a new
instance.

For example:

```sway
let some_slice = ...;
let buffer = Bytes::from(some_slice);
let mut buffer2 = buffer.clone();
// Now I can change `some_slice`
```

## Type Inference bug

I also had to fix a small problem with the type inference of method
applications. The problem is that sometimes the type check does not
return an error on the first pass, but the type is still not concrete.

For example `x.get(0) == Some(2)`, becomes `eq(x.get(0), Some(2))`.

After the first pass, `x.get(0)` is correctly inferred to be
`Option<u8>`; but `Some(2)` is only typed as `Option<Numeric>`. This
happens because the first pass starts with `TypeInfo::Unknown`. We can
use the argument types here because they may still be non-concrete types
like "Self".

What the fix is doing is that it checks if the inferred type
`is_concrete`, assuming that `Numeric` is not. This will make "Some(2)"
be type-checked again at the second pass, after monomorphization and be
correctly typed as "Option<u8>".

## IR Verification errors

This PR also improves the error message for IR verification. Now the
error is shown inside the printed IR.


![image](https://github.com/FuelLabs/sway/assets/83425/4f9eae39-0ce2-428d-be46-a215797ee8dd)

## Script to update contract-ids (optional)

At `test/update-contract-ids.sh` there is a small script that
automatically updates contract ids. Unfortunately, given the indirect
nature of contract calls, it is impossible for the script to know which
contract it needs to compile. So we need to specify the path to the
contract.

We also need to pass the compiler flags, because in some cases we use
`debug` profile, and in others we use `release`.

```sway
const FUEL_COIN_CONTRACT_ID = 0x1a88d0982d216958d18378b6784614b75868a542dc05f8cc85cf3da44268c76c; // AUTO-CONTRACT-ID ../../test_contracts/test_fuel_coin_contract --release
```

In the example above, the path and flags are appended in the bash
script. I failed trying to inject commands, so I think the append is
safe.

I can remove this from the PR, if we find that this is not the solution
we want at the moment.

## Checklist

- [ ] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [ ] I have requested a review from the relevant team or maintainers.
@kayagokalp
Copy link
Member

I did some hacking to see if i can generate asm for automatically generating loader: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=5ff55f234b0444e94599c5e505020ce3 With this simple asm generator I was able to replicate this test with 3 chunks.

@kayagokalp
Copy link
Member

I have a question though, is it fine to cut the contract wihtout any consideration. Meaning that is there any conditions that should be met for each chunk. Or any special case for the first or last chunk? Or can i replicate the split functionality?

@vaivaswatha
Copy link
Contributor Author

I have a question though, is it fine to cut the contract wihtout any consideration. Meaning that is there any conditions that should be met for each chunk. Or any special case for the first or last chunk? Or can i replicate the split functionality?

The VM appends each LDC'd chunk to a word boundary. So if your chunks aren't word aligned (i.e., size not divisible by 8), then it gets padded. The padding makes it bad for our purposes since the next chunk should be immediately after the currently LDC'd one.

So word aligning each chunk (except possibly for the last one) is necessary. I don't think there are any other constraints.

vaivaswatha and others added 6 commits July 9, 2024 11:54
## Description

When the input type was of `Input::Message` and the `msg_sender()`
function was called, it would revert. It now properly handle
`Input::Message`.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Kaya Gökalp <kaya.gokalp@fuel.sh>
Co-authored-by: SwayStar123 <46050679+SwayStar123@users.noreply.github.com>
Copy link

Benchmark for afbbc4d

Click to view benchmark
Test Base PR %
code_action 5.2±0.07ms 5.2±0.09ms 0.00%
code_lens 287.5±7.21ns 276.3±12.08ns -3.90%
compile 2.6±0.04s 2.6±0.03s 0.00%
completion 4.7±0.01ms 4.6±0.08ms -2.13%
did_change_with_caching 2.5±0.03s 2.5±0.03s 0.00%
document_symbol 868.4±17.97µs 846.8±40.56µs -2.49%
format 70.7±1.04ms 85.3±0.75ms +20.65%
goto_definition 337.5±5.65µs 336.2±3.41µs -0.39%
highlight 9.0±0.01ms 9.0±0.14ms 0.00%
hover 491.3±9.57µs 487.5±7.35µs -0.77%
idents_at_position 119.7±0.36µs 117.6±0.69µs -1.75%
inlay_hints 645.0±20.71µs 629.2±16.13µs -2.45%
on_enter 458.5±5.00ns 447.2±6.63ns -2.46%
parent_decl_at_position 3.8±0.16ms 3.7±0.03ms -2.63%
prepare_rename 337.9±7.27µs 337.5±6.77µs -0.12%
rename 9.3±0.15ms 9.2±0.12ms -1.08%
semantic_tokens 1228.5±13.45µs 1225.0±11.49µs -0.28%
token_at_position 329.6±1.73µs 359.5±1.83µs +9.07%
tokens_at_position 3.7±0.02ms 3.7±0.08ms 0.00%
tokens_for_file 398.9±3.16µs 402.3±2.70µs +0.85%
traverse 36.8±1.11ms 36.6±0.69ms -0.54%

@kayagokalp
Copy link
Member

I am closing this PR as this was mainly to showcase I believe and this feature is already in. If i am mistaken please re-open :)

@kayagokalp kayagokalp closed this Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants